Skip to content

fix: use server-side pagination for conference list tab#21

Merged
agonza1 merged 5 commits into
peermetrics:masterfrom
Boanerges1996:fix/server-side-pagination-dashboard
Apr 8, 2026
Merged

fix: use server-side pagination for conference list tab#21
agonza1 merged 5 commits into
peermetrics:masterfrom
Boanerges1996:fix/server-side-pagination-dashboard

Conversation

@Boanerges1996
Copy link
Copy Markdown
Contributor

Summary

Companion PR to peermetrics/api#19 — the API now supports ?limit=&offset= on list endpoints.

What changed

Conference list tab now uses server-side pagination:

  • Fetches 20 conferences per page via ?limit=20&offset=N
  • Page changes trigger a new API call instead of slicing in-memory arrays
  • Shows loading state while fetching
  • Uses totalCount from API response for pagination controls

Graphs tab is unchanged — still loads all conferences for charting. This is fast now because the API no longer inlines participants/issues on the list endpoint (N+1 eliminated in api#19).

Files changed

File Change
app-dashboard/components/app.vue Separate data for graphs (all) vs list (paginated), fetchConferences() method
app-dashboard/components/conferencesTab.vue Server-driven pagination, receives props from parent, emits page-changed event

Test plan

  • Dashboard graphs render correctly (uses full conference data)
  • Conference list tab shows 20 items per page
  • Clicking pagination loads next page from API (not client-side slicing)
  • Loading state appears during page fetch
  • First page loads fast (no more 56s wait)

Dependencies

The conference list tab now fetches one page at a time from the API
using ?limit=20&offset=N instead of loading all conferences into memory
and slicing with JavaScript.

- Graphs tab still loads all conferences (fast now without expand_fields)
- Conference list tab fetches paginated data via API
- Pagination controls emit page-changed event to trigger API fetch
- Loading state shown while fetching
- Backward compatible: graphs/filters still work with full conference data
The API list endpoint no longer returns issues (expand_fields removed
in api#19). The hasError/hasWarning helpers crashed when conference.issues
was undefined. Added null check so charts render without issues data.
The participant conferences tab accessed conference.issues.length
without checking if issues exists. With expand_fields removed from the
API list endpoint, this would crash.
@agonza1
Copy link
Copy Markdown
Contributor

agonza1 commented Apr 8, 2026

  1. could we keep the message when issues with conference? E.g: "We detected errors for this conference". We had that.
  2. page changes trigger a new API call instead of slicing in-memory arrays? Does this mean that we aren't using Redis for this? I think we probably should keep catching in Redis.
  3. Graphs tab should also have a default limit with indicator we can work on that here Add default 15-day date filter to ConferencesView api#16

The conference list was missing error/warning icons after expand_fields
was removed. Now uses has_errors and has_warnings boolean flags from the
API (annotated via Exists() subquery, no N+1).
All four API calls (issues, conferences, sessions, connections) now pass
created_at_gte using the daysHistory value (30 days). This prevents the
dashboard from loading months of historical data when the dataset grows.

Sessions endpoint already enforces a 30-day limit server-side, so the
param is redundant there but consistent.
@agonza1
Copy link
Copy Markdown
Contributor

agonza1 commented Apr 8, 2026

@codex review, does page changes trigger a new API call instead of slicing in-memory arrays? Does this mean that we aren't using Redis cache to optimize reads?

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ec4a631abb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +320 to +323
const response = await peermetrics.get(peermetrics.urls.conferences(), {
appId: peermetrics.app.id,
limit: this.conferencesPerPage,
offset: offset,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep pagination query scoped to dashboard history window

The new fetchConferences() request drops the created_at_gte filter that created() now applies to issues/sessions/connections and the full conference dataset, so the paginated conference tab can pull a different (all-time) dataset than the graphs and empty-state logic. On apps with older conferences, this causes inconsistent UI (e.g., graphs show only recent history while list/pagination counts include older rows) and unnecessary reads outside the intended time window.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the bot's concern is likely a false positive in our case.

The pagination is doing its job correctly:
The list shows X conferences at a time (one page) — that's expected
The charts show aggregated data across a broader dataset — also expected and intentional

These two views are supposed to show different scopes.

<div class="tab-pane fade" id="conferences" role="tabpanel" aria-labelledby="conferences-tab">
<conferences-tab :conferences="conferences" />
<conferences-tab
:conferences="paginatedConferences"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve dashboard filters for conference list tab

Binding the list tab to paginatedConferences bypasses applyFilters(), which still only updates this.conferences based on the selected app version/browser/OS/country watchers. After this change, changing filters updates graphs but leaves the conference list unfiltered, which is a behavior regression from the previous wiring and makes the global filter UI inconsistent across tabs.

Useful? React with 👍 / 👎.

@Boanerges1996
Copy link
Copy Markdown
Contributor Author

@agonza1 we are not using Redis, the redis we have is for mostly rate limiting. But I can make sure changes to use the redis to cache the data as well

@agonza1
Copy link
Copy Markdown
Contributor

agonza1 commented Apr 8, 2026

@agonza1 we are not using Redis, the redis we have is for mostly rate limiting. But I can make sure changes to use the redis to cache the data as well

@Boanerges1996 I believe cache is used on code paths that call Model.get(...) from BaseModel (basemodel.py). So not just rate limiting. If cache has the object, DB read is skipped. So this keeps that as it was which is good

Copy link
Copy Markdown
Contributor

@agonza1 agonza1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR intentionally keeps separate data paths:
-Graphs tab: full dataset
-Conference list tab: paginated dataset

@agonza1 agonza1 merged commit 0ae2a92 into peermetrics:master Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants